Feature/support mcp function meta extentions#2764
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 863c3aaa61
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| strict_json_schema=is_strict, | ||
| needs_approval=needs_approval, | ||
| mcp_title=resolve_mcp_tool_title(tool), | ||
| meta=tool.meta, |
There was a problem hiding this comment.
meta kwarg from MCP tool builder call
This call passes meta=tool.meta into _build_wrapped_function_tool, but that helper’s signature does not accept a meta parameter, so MCP tool conversion will raise TypeError at runtime when to_function_tool is executed. In practice, this breaks MCP tool registration/execution for any code path that converts tools through this helper.
Useful? React with 👍 / 👎.
| meta: dict[str, Any] | None = None | ||
| """Optional metadata for the tool.""" | ||
|
|
||
| defer_loading: bool = False |
There was a problem hiding this comment.
FunctionTool positional constructor compatibility
Adding meta before defer_loading changes the positional argument mapping for FunctionTool, so existing callers that passed defer_loading positionally will now populate meta instead and silently leave defer_loading at its default. Because FunctionTool is a public dataclass API, this is a backward-incompatible behavioral change and should be avoided by appending new optional fields at the end (or making them keyword-only).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 19c4e79c61
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| timeout_error_function=timeout_error_function, | ||
| defer_loading=defer_loading, | ||
| _mcp_title=mcp_title, | ||
| _meta=meta, |
There was a problem hiding this comment.
to_function_tool now passes meta=tool.meta, but _build_wrapped_function_tool only stores it on FunctionTool._meta and never binds it into invoke_tool_impl. Since invoke_function_tool calls on_invoke_tool(context, arguments) with no meta kwarg, MCPUtil.invoke_mcp_tool always sees meta=None and drops static MCP _meta from list-tools entries before server.call_tool(...). This breaks MCP servers/tools that depend on request metadata (for example auth/session hints) because converted tools silently omit that metadata at execution time.
Useful? React with 👍 / 👎.
|
I traced this locally and the smaller fix looks like it belongs in
The narrow path that worked locally was to bind the static MCP tool metadata at conversion time, e.g. by partially applying That also avoids changing I validated that direction locally with focused regressions for:
|
|
Closing this in favor off #2769 Thanks again for sharing this patch! |
3 participants
No description provided.